[Clang][CodeGen] Preserve alignment information for pointer arithmetics #152575
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Yingwei Zheng (dtcxzyw) ChangesPreviously, the alignment of pointer arithmetics was inferred from the pointee type, losing the alignment information from its operands: llvm-project/clang/lib/CodeGen/CGExpr.cpp Lines 1446 to 1449 in 503c090 This patch preserves alignment information for pointer arithmetics Closes #152330. Although the motivating case can be fixed by #145733, the alignment cannot be recovered without a dominating memory access with larger alignment. Patch is 21.00 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/152575.diff 6 Files Affected:
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 5a3d4e447b229..ba10f2ab19d4e 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -1319,6 +1319,57 @@ void CodeGenModule::EmitExplicitCastExprType(const ExplicitCastExpr *E,
// LValue Expression Emission
//===----------------------------------------------------------------------===//
+static CharUnits getArrayElementAlign(CharUnits arrayAlign, llvm::Value *idx,
+ CharUnits eltSize) {
+ // If we have a constant index, we can use the exact offset of the
+ // element we're accessing.
+ if (auto *constantIdx = dyn_cast<llvm::ConstantInt>(idx)) {
+ CharUnits offset = constantIdx->getZExtValue() * eltSize;
+ return arrayAlign.alignmentAtOffset(offset);
+ }
+
+ // Otherwise, use the worst-case alignment for any element.
+ return arrayAlign.alignmentOfArrayElement(eltSize);
+}
+
+/// Emit pointer + index arithmetic.
+static Address emitPointerArithmetic(CodeGenFunction &CGF,
+ const BinaryOperator *BO,
+ LValueBaseInfo *BaseInfo,
+ TBAAAccessInfo *TBAAInfo,
+ KnownNonNull_t IsKnownNonNull) {
+ assert(BO->isAdditiveOp() && "Expect an addition or subtraction.");
+ Expr *pointerOperand = BO->getLHS();
+ Expr *indexOperand = BO->getRHS();
+ bool isSubtraction = BO->getOpcode() == BO_Sub;
+
+ Address BaseAddr = Address::invalid();
+ llvm::Value *index = nullptr;
+ // In a subtraction, the LHS is always the pointer.
+ // Note: do not change the evaluation order.
+ if (!isSubtraction && !pointerOperand->getType()->isAnyPointerType()) {
+ std::swap(pointerOperand, indexOperand);
+ index = CGF.EmitScalarExpr(indexOperand);
+ BaseAddr = CGF.EmitPointerWithAlignment(pointerOperand, BaseInfo, TBAAInfo,
+ NotKnownNonNull);
+ } else {
+ BaseAddr = CGF.EmitPointerWithAlignment(pointerOperand, BaseInfo, TBAAInfo,
+ NotKnownNonNull);
+ index = CGF.EmitScalarExpr(indexOperand);
+ }
+
+ llvm::Value *pointer = BaseAddr.getBasePointer();
+ llvm::Value *Res = CGF.EmitPointerArithmetic(
+ BO, pointerOperand, pointer, indexOperand, index, isSubtraction);
+ QualType PointeeTy = BO->getType()->getPointeeType();
+ CharUnits Align =
+ getArrayElementAlign(BaseAddr.getAlignment(), index,
+ CGF.getContext().getTypeSizeInChars(PointeeTy));
+ return Address(Res, CGF.ConvertTypeForMem(PointeeTy), Align,
+ CGF.CGM.getPointerAuthInfoForPointeeType(PointeeTy),
+ /*Offset=*/nullptr, IsKnownNonNull);
+}
+
static Address EmitPointerWithAlignment(const Expr *E, LValueBaseInfo *BaseInfo,
TBAAAccessInfo *TBAAInfo,
KnownNonNull_t IsKnownNonNull,
@@ -1381,6 +1432,13 @@ static Address EmitPointerWithAlignment(const Expr *E, LValueBaseInfo *BaseInfo,
if (CE->getCastKind() == CK_AddressSpaceConversion)
Addr = CGF.Builder.CreateAddrSpaceCast(
Addr, CGF.ConvertType(E->getType()), ElemTy);
+ // Note: Workaround for PR114062. See also the special handling in
+ // ScalarExprEmitter::VisitCastExpr.
+ if (auto *A = dyn_cast<llvm::Argument>(Addr.getBasePointer());
+ A && A->hasStructRetAttr())
+ Addr = CGF.Builder.CreateAddrSpaceCast(
+ Addr, CGF.ConvertType(E->getType()), ElemTy);
+
return CGF.authPointerToPointerCast(Addr, CE->getSubExpr()->getType(),
CE->getType());
}
@@ -1441,6 +1499,12 @@ static Address EmitPointerWithAlignment(const Expr *E, LValueBaseInfo *BaseInfo,
}
}
+ // Pointer arithmetic: pointer +/- index.
+ if (auto *BO = dyn_cast<BinaryOperator>(E)) {
+ if (BO->isAdditiveOp())
+ return emitPointerArithmetic(CGF, BO, BaseInfo, TBAAInfo, IsKnownNonNull);
+ }
+
// TODO: conditional operators, comma.
// Otherwise, use the alignment of the type.
@@ -4210,21 +4274,6 @@ static Address emitArraySubscriptGEP(CodeGenFunction &CGF, Address addr,
}
}
-static CharUnits getArrayElementAlign(CharUnits arrayAlign,
- llvm::Value *idx,
- CharUnits eltSize) {
- // If we have a constant index, we can use the exact offset of the
- // element we're accessing.
- if (auto constantIdx = dyn_cast<llvm::ConstantInt>(idx)) {
- CharUnits offset = constantIdx->getZExtValue() * eltSize;
- return arrayAlign.alignmentAtOffset(offset);
-
- // Otherwise, use the worst-case alignment for any element.
- } else {
- return arrayAlign.alignmentOfArrayElement(eltSize);
- }
-}
-
static QualType getFixedSizeElementType(const ASTContext &ctx,
const VariableArrayType *vla) {
QualType eltType;
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index 44931d0481e26..9cd078bda4259 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -4184,29 +4184,14 @@ Value *ScalarExprEmitter::EmitOverflowCheckedBinOp(const BinOpInfo &Ops) {
}
/// Emit pointer + index arithmetic.
-static Value *emitPointerArithmetic(CodeGenFunction &CGF,
- const BinOpInfo &op,
- bool isSubtraction) {
- // Must have binary (not unary) expr here. Unary pointer
- // increment/decrement doesn't use this path.
- const BinaryOperator *expr = cast<BinaryOperator>(op.E);
-
- Value *pointer = op.LHS;
- Expr *pointerOperand = expr->getLHS();
- Value *index = op.RHS;
- Expr *indexOperand = expr->getRHS();
-
- // In a subtraction, the LHS is always the pointer.
- if (!isSubtraction && !pointer->getType()->isPointerTy()) {
- std::swap(pointer, index);
- std::swap(pointerOperand, indexOperand);
- }
-
+llvm::Value *CodeGenFunction::EmitPointerArithmetic(
+ const BinaryOperator *BO, Expr *pointerOperand, llvm::Value *pointer,
+ Expr *indexOperand, llvm::Value *index, bool isSubtraction) {
bool isSigned = indexOperand->getType()->isSignedIntegerOrEnumerationType();
unsigned width = cast<llvm::IntegerType>(index->getType())->getBitWidth();
- auto &DL = CGF.CGM.getDataLayout();
- auto PtrTy = cast<llvm::PointerType>(pointer->getType());
+ auto &DL = CGM.getDataLayout();
+ auto *PtrTy = cast<llvm::PointerType>(pointer->getType());
// Some versions of glibc and gcc use idioms (particularly in their malloc
// routines) that add a pointer-sized integer (known to be a pointer value)
@@ -4227,79 +4212,77 @@ static Value *emitPointerArithmetic(CodeGenFunction &CGF,
//
// Note that we do not suppress the pointer overflow check in this case.
if (BinaryOperator::isNullPointerArithmeticExtension(
- CGF.getContext(), op.Opcode, expr->getLHS(), expr->getRHS())) {
- Value *Ptr = CGF.Builder.CreateIntToPtr(index, pointer->getType());
- if (CGF.getLangOpts().PointerOverflowDefined ||
- !CGF.SanOpts.has(SanitizerKind::PointerOverflow) ||
- NullPointerIsDefined(CGF.Builder.GetInsertBlock()->getParent(),
+ getContext(), BO->getOpcode(), BO->getLHS(), BO->getRHS())) {
+ llvm::Value *Ptr = Builder.CreateIntToPtr(index, pointer->getType());
+ if (getLangOpts().PointerOverflowDefined ||
+ !SanOpts.has(SanitizerKind::PointerOverflow) ||
+ NullPointerIsDefined(Builder.GetInsertBlock()->getParent(),
PtrTy->getPointerAddressSpace()))
return Ptr;
// The inbounds GEP of null is valid iff the index is zero.
auto CheckOrdinal = SanitizerKind::SO_PointerOverflow;
auto CheckHandler = SanitizerHandler::PointerOverflow;
- SanitizerDebugLocation SanScope(&CGF, {CheckOrdinal}, CheckHandler);
- Value *IsZeroIndex = CGF.Builder.CreateIsNull(index);
- llvm::Constant *StaticArgs[] = {
- CGF.EmitCheckSourceLocation(op.E->getExprLoc())};
+ SanitizerDebugLocation SanScope(this, {CheckOrdinal}, CheckHandler);
+ llvm::Value *IsZeroIndex = Builder.CreateIsNull(index);
+ llvm::Constant *StaticArgs[] = {EmitCheckSourceLocation(BO->getExprLoc())};
llvm::Type *IntPtrTy = DL.getIntPtrType(PtrTy);
- Value *IntPtr = llvm::Constant::getNullValue(IntPtrTy);
- Value *ComputedGEP = CGF.Builder.CreateZExtOrTrunc(index, IntPtrTy);
- Value *DynamicArgs[] = {IntPtr, ComputedGEP};
- CGF.EmitCheck({{IsZeroIndex, CheckOrdinal}}, CheckHandler, StaticArgs,
- DynamicArgs);
+ llvm::Value *IntPtr = llvm::Constant::getNullValue(IntPtrTy);
+ llvm::Value *ComputedGEP = Builder.CreateZExtOrTrunc(index, IntPtrTy);
+ llvm::Value *DynamicArgs[] = {IntPtr, ComputedGEP};
+ EmitCheck({{IsZeroIndex, CheckOrdinal}}, CheckHandler, StaticArgs,
+ DynamicArgs);
return Ptr;
}
if (width != DL.getIndexTypeSizeInBits(PtrTy)) {
// Zero-extend or sign-extend the pointer value according to
// whether the index is signed or not.
- index = CGF.Builder.CreateIntCast(index, DL.getIndexType(PtrTy), isSigned,
- "idx.ext");
+ index = Builder.CreateIntCast(index, DL.getIndexType(PtrTy), isSigned,
+ "idx.ext");
}
// If this is subtraction, negate the index.
if (isSubtraction)
- index = CGF.Builder.CreateNeg(index, "idx.neg");
+ index = Builder.CreateNeg(index, "idx.neg");
- if (CGF.SanOpts.has(SanitizerKind::ArrayBounds))
- CGF.EmitBoundsCheck(op.E, pointerOperand, index, indexOperand->getType(),
- /*Accessed*/ false);
+ if (SanOpts.has(SanitizerKind::ArrayBounds))
+ EmitBoundsCheck(BO, pointerOperand, index, indexOperand->getType(),
+ /*Accessed*/ false);
- const PointerType *pointerType
- = pointerOperand->getType()->getAs<PointerType>();
+ const PointerType *pointerType =
+ pointerOperand->getType()->getAs<PointerType>();
if (!pointerType) {
QualType objectType = pointerOperand->getType()
- ->castAs<ObjCObjectPointerType>()
- ->getPointeeType();
- llvm::Value *objectSize
- = CGF.CGM.getSize(CGF.getContext().getTypeSizeInChars(objectType));
+ ->castAs<ObjCObjectPointerType>()
+ ->getPointeeType();
+ llvm::Value *objectSize =
+ CGM.getSize(getContext().getTypeSizeInChars(objectType));
- index = CGF.Builder.CreateMul(index, objectSize);
+ index = Builder.CreateMul(index, objectSize);
- Value *result =
- CGF.Builder.CreateGEP(CGF.Int8Ty, pointer, index, "add.ptr");
- return CGF.Builder.CreateBitCast(result, pointer->getType());
+ llvm::Value *result = Builder.CreateGEP(Int8Ty, pointer, index, "add.ptr");
+ return Builder.CreateBitCast(result, pointer->getType());
}
QualType elementType = pointerType->getPointeeType();
- if (const VariableArrayType *vla
- = CGF.getContext().getAsVariableArrayType(elementType)) {
+ if (const VariableArrayType *vla =
+ getContext().getAsVariableArrayType(elementType)) {
// The element count here is the total number of non-VLA elements.
- llvm::Value *numElements = CGF.getVLASize(vla).NumElts;
+ llvm::Value *numElements = getVLASize(vla).NumElts;
// Effectively, the multiply by the VLA size is part of the GEP.
// GEP indexes are signed, and scaling an index isn't permitted to
// signed-overflow, so we use the same semantics for our explicit
// multiply. We suppress this if overflow is not undefined behavior.
- llvm::Type *elemTy = CGF.ConvertTypeForMem(vla->getElementType());
- if (CGF.getLangOpts().PointerOverflowDefined) {
- index = CGF.Builder.CreateMul(index, numElements, "vla.index");
- pointer = CGF.Builder.CreateGEP(elemTy, pointer, index, "add.ptr");
+ llvm::Type *elemTy = ConvertTypeForMem(vla->getElementType());
+ if (getLangOpts().PointerOverflowDefined) {
+ index = Builder.CreateMul(index, numElements, "vla.index");
+ pointer = Builder.CreateGEP(elemTy, pointer, index, "add.ptr");
} else {
- index = CGF.Builder.CreateNSWMul(index, numElements, "vla.index");
- pointer = CGF.EmitCheckedInBoundsGEP(
- elemTy, pointer, index, isSigned, isSubtraction, op.E->getExprLoc(),
- "add.ptr");
+ index = Builder.CreateNSWMul(index, numElements, "vla.index");
+ pointer =
+ EmitCheckedInBoundsGEP(elemTy, pointer, index, isSigned,
+ isSubtraction, BO->getExprLoc(), "add.ptr");
}
return pointer;
}
@@ -4309,16 +4292,39 @@ static Value *emitPointerArithmetic(CodeGenFunction &CGF,
// future proof.
llvm::Type *elemTy;
if (elementType->isVoidType() || elementType->isFunctionType())
- elemTy = CGF.Int8Ty;
+ elemTy = Int8Ty;
else
- elemTy = CGF.ConvertTypeForMem(elementType);
+ elemTy = ConvertTypeForMem(elementType);
- if (CGF.getLangOpts().PointerOverflowDefined)
- return CGF.Builder.CreateGEP(elemTy, pointer, index, "add.ptr");
+ if (getLangOpts().PointerOverflowDefined)
+ return Builder.CreateGEP(elemTy, pointer, index, "add.ptr");
+
+ return EmitCheckedInBoundsGEP(elemTy, pointer, index, isSigned, isSubtraction,
+ BO->getExprLoc(), "add.ptr");
+}
+
+/// BO_Add/BO_Sub are handled by EmitPointerWithAlignment to preserve alignment
+/// information.
+/// This is a fallback path for BO_AddAssign/BO_SubAssign.
+static Value *emitPointerArithmetic(CodeGenFunction &CGF, const BinOpInfo &op,
+ bool isSubtraction) {
+ // Must have binary (not unary) expr here. Unary pointer
+ // increment/decrement doesn't use this path.
+ const BinaryOperator *expr = cast<BinaryOperator>(op.E);
+
+ Value *pointer = op.LHS;
+ Expr *pointerOperand = expr->getLHS();
+ Value *index = op.RHS;
+ Expr *indexOperand = expr->getRHS();
+
+ // In a subtraction, the LHS is always the pointer.
+ if (!isSubtraction && !pointer->getType()->isPointerTy()) {
+ std::swap(pointer, index);
+ std::swap(pointerOperand, indexOperand);
+ }
- return CGF.EmitCheckedInBoundsGEP(
- elemTy, pointer, index, isSigned, isSubtraction, op.E->getExprLoc(),
- "add.ptr");
+ return CGF.EmitPointerArithmetic(expr, pointerOperand, pointer, indexOperand,
+ index, isSubtraction);
}
// Construct an fmuladd intrinsic to represent a fused mul-add of MulOp and
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index 6c32c98cec011..103c8113a79de 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -5220,6 +5220,12 @@ class CodeGenFunction : public CodeGenTypeCache {
/// operation is a subtraction.
enum { NotSubtraction = false, IsSubtraction = true };
+ /// Emit pointer + index arithmetic.
+ llvm::Value *EmitPointerArithmetic(const BinaryOperator *BO,
+ Expr *pointerOperand, llvm::Value *pointer,
+ Expr *indexOperand, llvm::Value *index,
+ bool isSubtraction);
+
/// Same as IRBuilder::CreateInBoundsGEP, but additionally emits a check to
/// detect undefined behavior when the pointer overflow sanitizer is enabled.
/// \p SignedIndices indicates whether any of the GEP indices are signed.
diff --git a/clang/test/CodeGen/packed-arrays.c b/clang/test/CodeGen/packed-arrays.c
index 097fa7fc0feb7..51629b66d0687 100644
--- a/clang/test/CodeGen/packed-arrays.c
+++ b/clang/test/CodeGen/packed-arrays.c
@@ -55,7 +55,7 @@ int align3_x0 = __alignof(((struct s3*) 0)->x[0]);
// CHECK: load i32, ptr %{{.*}}, align 1
// CHECK: }
// CHECK-LABEL: define{{.*}} i32 @f0_b
-// CHECK: load i32, ptr %{{.*}}, align 4
+// CHECK: load i32, ptr %{{.*}}, align 1
// CHECK: }
int f0_a(struct s0 *a) {
return a->x[1];
@@ -100,7 +100,7 @@ int f1_d(struct s1 *a) {
// CHECK: load i32, ptr %{{.*}}, align 1
// CHECK: }
// CHECK-LABEL: define{{.*}} i32 @f2_b
-// CHECK: load i32, ptr %{{.*}}, align 4
+// CHECK: load i32, ptr %{{.*}}, align 1
// CHECK: }
// CHECK-LABEL: define{{.*}} i32 @f2_c
// CHECK: load i32, ptr %{{.*}}, align 1
@@ -125,7 +125,7 @@ int f2_d(struct s2 *a) {
// CHECK: load i32, ptr %{{.*}}, align 1
// CHECK: }
// CHECK-LABEL: define{{.*}} i32 @f3_b
-// CHECK: load i32, ptr %{{.*}}, align 4
+// CHECK: load i32, ptr %{{.*}}, align 1
// CHECK: }
// CHECK-LABEL: define{{.*}} i32 @f3_c
// CHECK: load i32, ptr %{{.*}}, align 1
diff --git a/clang/test/CodeGen/pointer-arithmetic-align.c b/clang/test/CodeGen/pointer-arithmetic-align.c
new file mode 100644
index 0000000000000..745ab84635c1b
--- /dev/null
+++ b/clang/test/CodeGen/pointer-arithmetic-align.c
@@ -0,0 +1,83 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --version 5
+// RUN: %clang_cc1 -O1 -triple=x86_64-unknown-linux %s -emit-llvm -o - | FileCheck %s
+
+typedef unsigned char uint8_t;
+typedef unsigned long long uint64_t;
+
+struct a {
+ uint64_t b;
+ uint8_t block[16];
+};
+
+// CHECK-LABEL: define dso_local void @ptradd_0(
+// CHECK-SAME: ptr noundef writeonly captures(none) initializes((8, 9)) [[CTX:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
+// CHECK-NEXT: [[ENTRY:.*:]]
+// CHECK-NEXT: [[BLOCK:%.*]] = getelementptr inbounds nuw i8, ptr [[CTX]], i64 8
+// CHECK-NEXT: store i8 0, ptr [[BLOCK]], align 8, !tbaa [[TBAA2:![0-9]+]]
+// CHECK-NEXT: ret void
+//
+void ptradd_0(struct a *ctx) {
+ *(ctx->block + 0) = 0;
+}
+
+// CHECK-LABEL: define dso_local void @ptradd_4(
+// CHECK-SAME: ptr noundef writeonly captures(none) initializes((12, 13)) [[CTX:%.*]]) local_unnamed_addr #[[ATTR0]] {
+// CHECK-NEXT: [[ENTRY:.*:]]
+// CHECK-NEXT: [[ADD_PTR:%.*]] = getelementptr inbounds nuw i8, ptr [[CTX]], i64 12
+// CHECK-NEXT: store i8 0, ptr [[ADD_PTR]], align 4, !tbaa [[TBAA2]]
+// CHECK-NEXT: ret void
+//
+void ptradd_4(struct a *ctx) {
+ *(ctx->block + 4) = 0;
+}
+
+// CHECK-LABEL: define dso_local void @ptradd_8(
+// CHECK-SAME: ptr noundef writeonly captures(none) initializes((16, 17)) [[CTX:%.*]]) local_unnamed_addr #[[ATTR0]] {
+// CHECK-NEXT: [[ENTRY:.*:]]
+// CHECK-NEXT: [[ADD_PTR:%.*]] = getelementptr inbounds nuw i8, ptr [[CTX]], i64 16
+// CHECK-NEXT: store i8 0, ptr [[ADD_PTR]], align 8, !tbaa [[TBAA2]]
+// CHECK-NEXT: ret void
+//
+void ptradd_8(struct a *ctx) {
+ *(ctx->block + 8) = 0;
+}
+
+// CHECK-LABEL: define dso_local void @ptradd_8_commuted(
+// CHECK-SAME: ptr noundef writeonly captures(none) initializes((16, 17)) [[CTX:%.*]]) local_unnamed_addr #[[ATTR0]] {
+// CHECK-NEXT: [[ENTRY:.*:]]
+// CHECK-NEXT: [[ADD_PTR:%.*]] = getelementptr inbounds nuw i8, ptr [[CTX]], i64 16
+// CHECK-NEXT: store i8 0, ptr [[ADD_PTR]], align 8, !tbaa [[TBAA2]]
+// CHECK-NEXT: ret void
+//
+void ptradd_8_commuted(struct a *ctx) {
+ *(8 + ctx->block) = 0;
+}
+
+// CHECK-LABEL: define dso_local void @ptrsub_4(
+// CHECK-SAME: ptr noundef writeonly captures(none) initializes((8, 9)) [[CTX:%.*]]) local_unnamed_addr #[[ATTR0]] {
+// CHECK-NEXT: [[ENTRY:.*:]]
+// CHECK-NEXT: [[ADD_PTR:%.*]] = getelementptr inbounds nuw i8, ptr [[CTX]], i64 8
+// CHECK-NEXT: store i8 0, ptr [[ADD_PTR]], align 4, !tbaa [[TBAA2]]
+// CHECK-NEXT: ret void
+//
+void ptrsub_4(struct a *ctx) {
+ *(&ctx->block[4] - 4) = 0;
+}
+
+// CHECK-LABEL: define dso_local void @neg_ptradd_var_index(
+// CHECK-SAME: ptr noundef writeonly captures(none) [[CTX:%.*]], i8 noundef zeroext [[IDX:%.*]]) local_unnamed_addr #[[ATTR0]] {
+// CHECK-NEXT: [[ENTRY:.*:]]
+// CHECK-NEXT: [[BLOCK:%.*]] = getelementptr inbounds nuw i8, ptr [[CTX]], i64 8
+// CHECK-NEXT: [[IDX_EXT:%.*]] = zext i8 [[IDX]] to i64
+// CHECK-NEXT: [[ADD_PTR:%.*]] = getelementptr inbounds nuw i8, ptr [[BLOCK]], i64 [[IDX_EXT]]
+// CHECK-NEXT: store i8 0, ptr [[ADD_PTR]], align 1, !tbaa [[TBAA2]]
+// CHECK-NEXT: ret void
+//
+void neg_ptradd_var_index(struct a *ctx, uint8_t idx) {
+ *(ctx-...
[truncated]
|
|
@llvm/pr-subscribers-clang-codegen Author: Yingwei Zheng (dtcxzyw) ChangesPreviously, the alignment of pointer arithmetics was inferred from the pointee type, losing the alignment information from its operands: llvm-project/clang/lib/CodeGen/CGExpr.cpp Lines 1446 to 1449 in 503c090 This patch preserves alignment information for pointer arithmetics Closes #152330. Although the motivating case can be fixed by #145733, the alignment cannot be recovered without a dominating memory access with larger alignment. Patch is 21.00 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/152575.diff 6 Files Affected:
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 5a3d4e447b229..ba10f2ab19d4e 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -1319,6 +1319,57 @@ void CodeGenModule::EmitExplicitCastExprType(const ExplicitCastExpr *E,
// LValue Expression Emission
//===----------------------------------------------------------------------===//
+static CharUnits getArrayElementAlign(CharUnits arrayAlign, llvm::Value *idx,
+ CharUnits eltSize) {
+ // If we have a constant index, we can use the exact offset of the
+ // element we're accessing.
+ if (auto *constantIdx = dyn_cast<llvm::ConstantInt>(idx)) {
+ CharUnits offset = constantIdx->getZExtValue() * eltSize;
+ return arrayAlign.alignmentAtOffset(offset);
+ }
+
+ // Otherwise, use the worst-case alignment for any element.
+ return arrayAlign.alignmentOfArrayElement(eltSize);
+}
+
+/// Emit pointer + index arithmetic.
+static Address emitPointerArithmetic(CodeGenFunction &CGF,
+ const BinaryOperator *BO,
+ LValueBaseInfo *BaseInfo,
+ TBAAAccessInfo *TBAAInfo,
+ KnownNonNull_t IsKnownNonNull) {
+ assert(BO->isAdditiveOp() && "Expect an addition or subtraction.");
+ Expr *pointerOperand = BO->getLHS();
+ Expr *indexOperand = BO->getRHS();
+ bool isSubtraction = BO->getOpcode() == BO_Sub;
+
+ Address BaseAddr = Address::invalid();
+ llvm::Value *index = nullptr;
+ // In a subtraction, the LHS is always the pointer.
+ // Note: do not change the evaluation order.
+ if (!isSubtraction && !pointerOperand->getType()->isAnyPointerType()) {
+ std::swap(pointerOperand, indexOperand);
+ index = CGF.EmitScalarExpr(indexOperand);
+ BaseAddr = CGF.EmitPointerWithAlignment(pointerOperand, BaseInfo, TBAAInfo,
+ NotKnownNonNull);
+ } else {
+ BaseAddr = CGF.EmitPointerWithAlignment(pointerOperand, BaseInfo, TBAAInfo,
+ NotKnownNonNull);
+ index = CGF.EmitScalarExpr(indexOperand);
+ }
+
+ llvm::Value *pointer = BaseAddr.getBasePointer();
+ llvm::Value *Res = CGF.EmitPointerArithmetic(
+ BO, pointerOperand, pointer, indexOperand, index, isSubtraction);
+ QualType PointeeTy = BO->getType()->getPointeeType();
+ CharUnits Align =
+ getArrayElementAlign(BaseAddr.getAlignment(), index,
+ CGF.getContext().getTypeSizeInChars(PointeeTy));
+ return Address(Res, CGF.ConvertTypeForMem(PointeeTy), Align,
+ CGF.CGM.getPointerAuthInfoForPointeeType(PointeeTy),
+ /*Offset=*/nullptr, IsKnownNonNull);
+}
+
static Address EmitPointerWithAlignment(const Expr *E, LValueBaseInfo *BaseInfo,
TBAAAccessInfo *TBAAInfo,
KnownNonNull_t IsKnownNonNull,
@@ -1381,6 +1432,13 @@ static Address EmitPointerWithAlignment(const Expr *E, LValueBaseInfo *BaseInfo,
if (CE->getCastKind() == CK_AddressSpaceConversion)
Addr = CGF.Builder.CreateAddrSpaceCast(
Addr, CGF.ConvertType(E->getType()), ElemTy);
+ // Note: Workaround for PR114062. See also the special handling in
+ // ScalarExprEmitter::VisitCastExpr.
+ if (auto *A = dyn_cast<llvm::Argument>(Addr.getBasePointer());
+ A && A->hasStructRetAttr())
+ Addr = CGF.Builder.CreateAddrSpaceCast(
+ Addr, CGF.ConvertType(E->getType()), ElemTy);
+
return CGF.authPointerToPointerCast(Addr, CE->getSubExpr()->getType(),
CE->getType());
}
@@ -1441,6 +1499,12 @@ static Address EmitPointerWithAlignment(const Expr *E, LValueBaseInfo *BaseInfo,
}
}
+ // Pointer arithmetic: pointer +/- index.
+ if (auto *BO = dyn_cast<BinaryOperator>(E)) {
+ if (BO->isAdditiveOp())
+ return emitPointerArithmetic(CGF, BO, BaseInfo, TBAAInfo, IsKnownNonNull);
+ }
+
// TODO: conditional operators, comma.
// Otherwise, use the alignment of the type.
@@ -4210,21 +4274,6 @@ static Address emitArraySubscriptGEP(CodeGenFunction &CGF, Address addr,
}
}
-static CharUnits getArrayElementAlign(CharUnits arrayAlign,
- llvm::Value *idx,
- CharUnits eltSize) {
- // If we have a constant index, we can use the exact offset of the
- // element we're accessing.
- if (auto constantIdx = dyn_cast<llvm::ConstantInt>(idx)) {
- CharUnits offset = constantIdx->getZExtValue() * eltSize;
- return arrayAlign.alignmentAtOffset(offset);
-
- // Otherwise, use the worst-case alignment for any element.
- } else {
- return arrayAlign.alignmentOfArrayElement(eltSize);
- }
-}
-
static QualType getFixedSizeElementType(const ASTContext &ctx,
const VariableArrayType *vla) {
QualType eltType;
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index 44931d0481e26..9cd078bda4259 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -4184,29 +4184,14 @@ Value *ScalarExprEmitter::EmitOverflowCheckedBinOp(const BinOpInfo &Ops) {
}
/// Emit pointer + index arithmetic.
-static Value *emitPointerArithmetic(CodeGenFunction &CGF,
- const BinOpInfo &op,
- bool isSubtraction) {
- // Must have binary (not unary) expr here. Unary pointer
- // increment/decrement doesn't use this path.
- const BinaryOperator *expr = cast<BinaryOperator>(op.E);
-
- Value *pointer = op.LHS;
- Expr *pointerOperand = expr->getLHS();
- Value *index = op.RHS;
- Expr *indexOperand = expr->getRHS();
-
- // In a subtraction, the LHS is always the pointer.
- if (!isSubtraction && !pointer->getType()->isPointerTy()) {
- std::swap(pointer, index);
- std::swap(pointerOperand, indexOperand);
- }
-
+llvm::Value *CodeGenFunction::EmitPointerArithmetic(
+ const BinaryOperator *BO, Expr *pointerOperand, llvm::Value *pointer,
+ Expr *indexOperand, llvm::Value *index, bool isSubtraction) {
bool isSigned = indexOperand->getType()->isSignedIntegerOrEnumerationType();
unsigned width = cast<llvm::IntegerType>(index->getType())->getBitWidth();
- auto &DL = CGF.CGM.getDataLayout();
- auto PtrTy = cast<llvm::PointerType>(pointer->getType());
+ auto &DL = CGM.getDataLayout();
+ auto *PtrTy = cast<llvm::PointerType>(pointer->getType());
// Some versions of glibc and gcc use idioms (particularly in their malloc
// routines) that add a pointer-sized integer (known to be a pointer value)
@@ -4227,79 +4212,77 @@ static Value *emitPointerArithmetic(CodeGenFunction &CGF,
//
// Note that we do not suppress the pointer overflow check in this case.
if (BinaryOperator::isNullPointerArithmeticExtension(
- CGF.getContext(), op.Opcode, expr->getLHS(), expr->getRHS())) {
- Value *Ptr = CGF.Builder.CreateIntToPtr(index, pointer->getType());
- if (CGF.getLangOpts().PointerOverflowDefined ||
- !CGF.SanOpts.has(SanitizerKind::PointerOverflow) ||
- NullPointerIsDefined(CGF.Builder.GetInsertBlock()->getParent(),
+ getContext(), BO->getOpcode(), BO->getLHS(), BO->getRHS())) {
+ llvm::Value *Ptr = Builder.CreateIntToPtr(index, pointer->getType());
+ if (getLangOpts().PointerOverflowDefined ||
+ !SanOpts.has(SanitizerKind::PointerOverflow) ||
+ NullPointerIsDefined(Builder.GetInsertBlock()->getParent(),
PtrTy->getPointerAddressSpace()))
return Ptr;
// The inbounds GEP of null is valid iff the index is zero.
auto CheckOrdinal = SanitizerKind::SO_PointerOverflow;
auto CheckHandler = SanitizerHandler::PointerOverflow;
- SanitizerDebugLocation SanScope(&CGF, {CheckOrdinal}, CheckHandler);
- Value *IsZeroIndex = CGF.Builder.CreateIsNull(index);
- llvm::Constant *StaticArgs[] = {
- CGF.EmitCheckSourceLocation(op.E->getExprLoc())};
+ SanitizerDebugLocation SanScope(this, {CheckOrdinal}, CheckHandler);
+ llvm::Value *IsZeroIndex = Builder.CreateIsNull(index);
+ llvm::Constant *StaticArgs[] = {EmitCheckSourceLocation(BO->getExprLoc())};
llvm::Type *IntPtrTy = DL.getIntPtrType(PtrTy);
- Value *IntPtr = llvm::Constant::getNullValue(IntPtrTy);
- Value *ComputedGEP = CGF.Builder.CreateZExtOrTrunc(index, IntPtrTy);
- Value *DynamicArgs[] = {IntPtr, ComputedGEP};
- CGF.EmitCheck({{IsZeroIndex, CheckOrdinal}}, CheckHandler, StaticArgs,
- DynamicArgs);
+ llvm::Value *IntPtr = llvm::Constant::getNullValue(IntPtrTy);
+ llvm::Value *ComputedGEP = Builder.CreateZExtOrTrunc(index, IntPtrTy);
+ llvm::Value *DynamicArgs[] = {IntPtr, ComputedGEP};
+ EmitCheck({{IsZeroIndex, CheckOrdinal}}, CheckHandler, StaticArgs,
+ DynamicArgs);
return Ptr;
}
if (width != DL.getIndexTypeSizeInBits(PtrTy)) {
// Zero-extend or sign-extend the pointer value according to
// whether the index is signed or not.
- index = CGF.Builder.CreateIntCast(index, DL.getIndexType(PtrTy), isSigned,
- "idx.ext");
+ index = Builder.CreateIntCast(index, DL.getIndexType(PtrTy), isSigned,
+ "idx.ext");
}
// If this is subtraction, negate the index.
if (isSubtraction)
- index = CGF.Builder.CreateNeg(index, "idx.neg");
+ index = Builder.CreateNeg(index, "idx.neg");
- if (CGF.SanOpts.has(SanitizerKind::ArrayBounds))
- CGF.EmitBoundsCheck(op.E, pointerOperand, index, indexOperand->getType(),
- /*Accessed*/ false);
+ if (SanOpts.has(SanitizerKind::ArrayBounds))
+ EmitBoundsCheck(BO, pointerOperand, index, indexOperand->getType(),
+ /*Accessed*/ false);
- const PointerType *pointerType
- = pointerOperand->getType()->getAs<PointerType>();
+ const PointerType *pointerType =
+ pointerOperand->getType()->getAs<PointerType>();
if (!pointerType) {
QualType objectType = pointerOperand->getType()
- ->castAs<ObjCObjectPointerType>()
- ->getPointeeType();
- llvm::Value *objectSize
- = CGF.CGM.getSize(CGF.getContext().getTypeSizeInChars(objectType));
+ ->castAs<ObjCObjectPointerType>()
+ ->getPointeeType();
+ llvm::Value *objectSize =
+ CGM.getSize(getContext().getTypeSizeInChars(objectType));
- index = CGF.Builder.CreateMul(index, objectSize);
+ index = Builder.CreateMul(index, objectSize);
- Value *result =
- CGF.Builder.CreateGEP(CGF.Int8Ty, pointer, index, "add.ptr");
- return CGF.Builder.CreateBitCast(result, pointer->getType());
+ llvm::Value *result = Builder.CreateGEP(Int8Ty, pointer, index, "add.ptr");
+ return Builder.CreateBitCast(result, pointer->getType());
}
QualType elementType = pointerType->getPointeeType();
- if (const VariableArrayType *vla
- = CGF.getContext().getAsVariableArrayType(elementType)) {
+ if (const VariableArrayType *vla =
+ getContext().getAsVariableArrayType(elementType)) {
// The element count here is the total number of non-VLA elements.
- llvm::Value *numElements = CGF.getVLASize(vla).NumElts;
+ llvm::Value *numElements = getVLASize(vla).NumElts;
// Effectively, the multiply by the VLA size is part of the GEP.
// GEP indexes are signed, and scaling an index isn't permitted to
// signed-overflow, so we use the same semantics for our explicit
// multiply. We suppress this if overflow is not undefined behavior.
- llvm::Type *elemTy = CGF.ConvertTypeForMem(vla->getElementType());
- if (CGF.getLangOpts().PointerOverflowDefined) {
- index = CGF.Builder.CreateMul(index, numElements, "vla.index");
- pointer = CGF.Builder.CreateGEP(elemTy, pointer, index, "add.ptr");
+ llvm::Type *elemTy = ConvertTypeForMem(vla->getElementType());
+ if (getLangOpts().PointerOverflowDefined) {
+ index = Builder.CreateMul(index, numElements, "vla.index");
+ pointer = Builder.CreateGEP(elemTy, pointer, index, "add.ptr");
} else {
- index = CGF.Builder.CreateNSWMul(index, numElements, "vla.index");
- pointer = CGF.EmitCheckedInBoundsGEP(
- elemTy, pointer, index, isSigned, isSubtraction, op.E->getExprLoc(),
- "add.ptr");
+ index = Builder.CreateNSWMul(index, numElements, "vla.index");
+ pointer =
+ EmitCheckedInBoundsGEP(elemTy, pointer, index, isSigned,
+ isSubtraction, BO->getExprLoc(), "add.ptr");
}
return pointer;
}
@@ -4309,16 +4292,39 @@ static Value *emitPointerArithmetic(CodeGenFunction &CGF,
// future proof.
llvm::Type *elemTy;
if (elementType->isVoidType() || elementType->isFunctionType())
- elemTy = CGF.Int8Ty;
+ elemTy = Int8Ty;
else
- elemTy = CGF.ConvertTypeForMem(elementType);
+ elemTy = ConvertTypeForMem(elementType);
- if (CGF.getLangOpts().PointerOverflowDefined)
- return CGF.Builder.CreateGEP(elemTy, pointer, index, "add.ptr");
+ if (getLangOpts().PointerOverflowDefined)
+ return Builder.CreateGEP(elemTy, pointer, index, "add.ptr");
+
+ return EmitCheckedInBoundsGEP(elemTy, pointer, index, isSigned, isSubtraction,
+ BO->getExprLoc(), "add.ptr");
+}
+
+/// BO_Add/BO_Sub are handled by EmitPointerWithAlignment to preserve alignment
+/// information.
+/// This is a fallback path for BO_AddAssign/BO_SubAssign.
+static Value *emitPointerArithmetic(CodeGenFunction &CGF, const BinOpInfo &op,
+ bool isSubtraction) {
+ // Must have binary (not unary) expr here. Unary pointer
+ // increment/decrement doesn't use this path.
+ const BinaryOperator *expr = cast<BinaryOperator>(op.E);
+
+ Value *pointer = op.LHS;
+ Expr *pointerOperand = expr->getLHS();
+ Value *index = op.RHS;
+ Expr *indexOperand = expr->getRHS();
+
+ // In a subtraction, the LHS is always the pointer.
+ if (!isSubtraction && !pointer->getType()->isPointerTy()) {
+ std::swap(pointer, index);
+ std::swap(pointerOperand, indexOperand);
+ }
- return CGF.EmitCheckedInBoundsGEP(
- elemTy, pointer, index, isSigned, isSubtraction, op.E->getExprLoc(),
- "add.ptr");
+ return CGF.EmitPointerArithmetic(expr, pointerOperand, pointer, indexOperand,
+ index, isSubtraction);
}
// Construct an fmuladd intrinsic to represent a fused mul-add of MulOp and
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index 6c32c98cec011..103c8113a79de 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -5220,6 +5220,12 @@ class CodeGenFunction : public CodeGenTypeCache {
/// operation is a subtraction.
enum { NotSubtraction = false, IsSubtraction = true };
+ /// Emit pointer + index arithmetic.
+ llvm::Value *EmitPointerArithmetic(const BinaryOperator *BO,
+ Expr *pointerOperand, llvm::Value *pointer,
+ Expr *indexOperand, llvm::Value *index,
+ bool isSubtraction);
+
/// Same as IRBuilder::CreateInBoundsGEP, but additionally emits a check to
/// detect undefined behavior when the pointer overflow sanitizer is enabled.
/// \p SignedIndices indicates whether any of the GEP indices are signed.
diff --git a/clang/test/CodeGen/packed-arrays.c b/clang/test/CodeGen/packed-arrays.c
index 097fa7fc0feb7..51629b66d0687 100644
--- a/clang/test/CodeGen/packed-arrays.c
+++ b/clang/test/CodeGen/packed-arrays.c
@@ -55,7 +55,7 @@ int align3_x0 = __alignof(((struct s3*) 0)->x[0]);
// CHECK: load i32, ptr %{{.*}}, align 1
// CHECK: }
// CHECK-LABEL: define{{.*}} i32 @f0_b
-// CHECK: load i32, ptr %{{.*}}, align 4
+// CHECK: load i32, ptr %{{.*}}, align 1
// CHECK: }
int f0_a(struct s0 *a) {
return a->x[1];
@@ -100,7 +100,7 @@ int f1_d(struct s1 *a) {
// CHECK: load i32, ptr %{{.*}}, align 1
// CHECK: }
// CHECK-LABEL: define{{.*}} i32 @f2_b
-// CHECK: load i32, ptr %{{.*}}, align 4
+// CHECK: load i32, ptr %{{.*}}, align 1
// CHECK: }
// CHECK-LABEL: define{{.*}} i32 @f2_c
// CHECK: load i32, ptr %{{.*}}, align 1
@@ -125,7 +125,7 @@ int f2_d(struct s2 *a) {
// CHECK: load i32, ptr %{{.*}}, align 1
// CHECK: }
// CHECK-LABEL: define{{.*}} i32 @f3_b
-// CHECK: load i32, ptr %{{.*}}, align 4
+// CHECK: load i32, ptr %{{.*}}, align 1
// CHECK: }
// CHECK-LABEL: define{{.*}} i32 @f3_c
// CHECK: load i32, ptr %{{.*}}, align 1
diff --git a/clang/test/CodeGen/pointer-arithmetic-align.c b/clang/test/CodeGen/pointer-arithmetic-align.c
new file mode 100644
index 0000000000000..745ab84635c1b
--- /dev/null
+++ b/clang/test/CodeGen/pointer-arithmetic-align.c
@@ -0,0 +1,83 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --version 5
+// RUN: %clang_cc1 -O1 -triple=x86_64-unknown-linux %s -emit-llvm -o - | FileCheck %s
+
+typedef unsigned char uint8_t;
+typedef unsigned long long uint64_t;
+
+struct a {
+ uint64_t b;
+ uint8_t block[16];
+};
+
+// CHECK-LABEL: define dso_local void @ptradd_0(
+// CHECK-SAME: ptr noundef writeonly captures(none) initializes((8, 9)) [[CTX:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
+// CHECK-NEXT: [[ENTRY:.*:]]
+// CHECK-NEXT: [[BLOCK:%.*]] = getelementptr inbounds nuw i8, ptr [[CTX]], i64 8
+// CHECK-NEXT: store i8 0, ptr [[BLOCK]], align 8, !tbaa [[TBAA2:![0-9]+]]
+// CHECK-NEXT: ret void
+//
+void ptradd_0(struct a *ctx) {
+ *(ctx->block + 0) = 0;
+}
+
+// CHECK-LABEL: define dso_local void @ptradd_4(
+// CHECK-SAME: ptr noundef writeonly captures(none) initializes((12, 13)) [[CTX:%.*]]) local_unnamed_addr #[[ATTR0]] {
+// CHECK-NEXT: [[ENTRY:.*:]]
+// CHECK-NEXT: [[ADD_PTR:%.*]] = getelementptr inbounds nuw i8, ptr [[CTX]], i64 12
+// CHECK-NEXT: store i8 0, ptr [[ADD_PTR]], align 4, !tbaa [[TBAA2]]
+// CHECK-NEXT: ret void
+//
+void ptradd_4(struct a *ctx) {
+ *(ctx->block + 4) = 0;
+}
+
+// CHECK-LABEL: define dso_local void @ptradd_8(
+// CHECK-SAME: ptr noundef writeonly captures(none) initializes((16, 17)) [[CTX:%.*]]) local_unnamed_addr #[[ATTR0]] {
+// CHECK-NEXT: [[ENTRY:.*:]]
+// CHECK-NEXT: [[ADD_PTR:%.*]] = getelementptr inbounds nuw i8, ptr [[CTX]], i64 16
+// CHECK-NEXT: store i8 0, ptr [[ADD_PTR]], align 8, !tbaa [[TBAA2]]
+// CHECK-NEXT: ret void
+//
+void ptradd_8(struct a *ctx) {
+ *(ctx->block + 8) = 0;
+}
+
+// CHECK-LABEL: define dso_local void @ptradd_8_commuted(
+// CHECK-SAME: ptr noundef writeonly captures(none) initializes((16, 17)) [[CTX:%.*]]) local_unnamed_addr #[[ATTR0]] {
+// CHECK-NEXT: [[ENTRY:.*:]]
+// CHECK-NEXT: [[ADD_PTR:%.*]] = getelementptr inbounds nuw i8, ptr [[CTX]], i64 16
+// CHECK-NEXT: store i8 0, ptr [[ADD_PTR]], align 8, !tbaa [[TBAA2]]
+// CHECK-NEXT: ret void
+//
+void ptradd_8_commuted(struct a *ctx) {
+ *(8 + ctx->block) = 0;
+}
+
+// CHECK-LABEL: define dso_local void @ptrsub_4(
+// CHECK-SAME: ptr noundef writeonly captures(none) initializes((8, 9)) [[CTX:%.*]]) local_unnamed_addr #[[ATTR0]] {
+// CHECK-NEXT: [[ENTRY:.*:]]
+// CHECK-NEXT: [[ADD_PTR:%.*]] = getelementptr inbounds nuw i8, ptr [[CTX]], i64 8
+// CHECK-NEXT: store i8 0, ptr [[ADD_PTR]], align 4, !tbaa [[TBAA2]]
+// CHECK-NEXT: ret void
+//
+void ptrsub_4(struct a *ctx) {
+ *(&ctx->block[4] - 4) = 0;
+}
+
+// CHECK-LABEL: define dso_local void @neg_ptradd_var_index(
+// CHECK-SAME: ptr noundef writeonly captures(none) [[CTX:%.*]], i8 noundef zeroext [[IDX:%.*]]) local_unnamed_addr #[[ATTR0]] {
+// CHECK-NEXT: [[ENTRY:.*:]]
+// CHECK-NEXT: [[BLOCK:%.*]] = getelementptr inbounds nuw i8, ptr [[CTX]], i64 8
+// CHECK-NEXT: [[IDX_EXT:%.*]] = zext i8 [[IDX]] to i64
+// CHECK-NEXT: [[ADD_PTR:%.*]] = getelementptr inbounds nuw i8, ptr [[BLOCK]], i64 [[IDX_EXT]]
+// CHECK-NEXT: store i8 0, ptr [[ADD_PTR]], align 1, !tbaa [[TBAA2]]
+// CHECK-NEXT: ret void
+//
+void neg_ptradd_var_index(struct a *ctx, uint8_t idx) {
+ *(ctx-...
[truncated]
|
dtcxzyw
commented
Aug 7, 2025
| @@ -55,7 +55,7 @@ int align3_x0 = __alignof(((struct s3*) 0)->x[0]); | |||
| // CHECK: load i32, ptr %{{.*}}, align 1 | |||
| // CHECK: } | |||
| // CHECK-LABEL: define{{.*}} i32 @f0_b | |||
| // CHECK: load i32, ptr %{{.*}}, align 4 | |||
| // CHECK: load i32, ptr %{{.*}}, align 1 | |||
There was a problem hiding this comment.
It matches GCC's behavior: https://godbolt.org/z/TeGqTY1qc
| @@ -1381,6 +1432,13 @@ static Address EmitPointerWithAlignment(const Expr *E, LValueBaseInfo *BaseInfo, | |||
| if (CE->getCastKind() == CK_AddressSpaceConversion) | |||
| Addr = CGF.Builder.CreateAddrSpaceCast( | |||
| Addr, CGF.ConvertType(E->getType()), ElemTy); | |||
| // Note: Workaround for PR114062. See also the special handling in | |||
There was a problem hiding this comment.
Inserting random casts is how we get into trouble like this in the first place...
dtcxzyw
force-pushed
the
perf/ptr-arith-alignment
branch
from
be202cd to
e58f835
Compare
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Aug 8, 2025
…nFunction`. NFC. (#152634) `CodeGenFunction::EmitPointerArithmetic` is needed by llvm/llvm-project#152575. Separate the NFC changes into a new PR for smooth review.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, the alignment of pointer arithmetics was inferred from the pointee type, losing the alignment information from its operands:
llvm-project/clang/lib/CodeGen/CGExpr.cpp
Lines 1446 to 1449 in 503c090
This patch preserves alignment information for pointer arithmetics
P +/- C, to match the behavior of identical array subscript&P[C]: https://godbolt.org/z/xx1hfTrx4.Closes #152330. Although the motivating case can be fixed by #145733, the alignment cannot be recovered without a dominating memory access with larger alignment.